Ensure selfcheck file inherits directory permissions #13528
+35
−14
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amol-
force-pushed
the
selfcheck-permissions
branch
from
1db6780 to
2e4aaa2
Compare
|
Hi @amol- thanks for your PR to pip. Please be aware it take the maintainers some time to review as we are all doing this on a volunteer basis. |
ichard26
requested changes
Aug 13, 2025
| @@ -0,0 +1 @@ | |||
| selfcheck file in cache directory has same permissions as the rest of the cache. | |||
There was a problem hiding this comment.
Suggested change
| selfcheck file in cache directory has same permissions as the rest of the cache. | |
| Ensure the self-check files in the cache has same permissions as the rest of the cache. |
Comment on lines
+157
to
+160
| os.stat(directory).st_mode | ||
| & 0o666 # select read/write permissions of cache directory | ||
| | 0o600 # set owner read/write permissions | ||
| ) |
There was a problem hiding this comment.
Suggested change
| os.stat(directory).st_mode | |
| & 0o666 # select read/write permissions of cache directory | |
| | 0o600 # set owner read/write permissions | |
| ) | |
| os.stat(directory).st_mode | |
| & 0o666 # select read/write permissions of directory | |
| | 0o600 # set owner read/write permissions | |
| ) |
Comment on lines
192
to
+195
|
|
||
| statefile_permissions = os.stat(expected_path).st_mode & 0o666 | ||
| selfcheckdir_permissions = os.stat(cache_dir / "selfcheck").st_mode & 0o666 | ||
| assert statefile_permissions == selfcheckdir_permissions |
There was a problem hiding this comment.
Suggested change
| statefile_permissions = os.stat(expected_path).st_mode & 0o666 | |
| selfcheckdir_permissions = os.stat(cache_dir / "selfcheck").st_mode & 0o666 | |
| assert statefile_permissions == selfcheckdir_permissions | |
| # Check that the self-check cache entries inherit the root cache permissions. | |
| statefile_permissions = os.stat(expected_path).st_mode & 0o666 | |
| selfcheckdir_permissions = os.stat(cache_dir / "selfcheck").st_mode & 0o666 | |
| cache_permissions = os.stat(cache_dir).st_mode & 0o666 | |
| assert statefile_permissions == selfcheckdir_permissions == cache_permissions |
Let's add a comment and also check against the root .cache folder permissions as that's how the inheritance works with the network cache.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When writing files to cache pip does copy the permissions from the cache directory, this is an expected behaviour to ensure that any user that has access to the cache has also access to its content.
Copying the permissions explicitly is necessary due to the use of
adjacent_tmp_filewhich by virtue oftempfile.mkstemp(indirectly viaNamedTemporaryFile) causes all files to be created with600permissions.This is not happening for
selfcheck/XXXXXby the way, which makes the selfcheck unaccessible by users different from the one that created the cache.This PullRequest creates an helper method
copy_directory_permissionswhich allows both the cache and the selfcheck file to share the same logic in copying directory permissions and thus ensure consistency in cache files permissions.